Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transition to API version 0.0.4 #6944

Merged
merged 22 commits into from
Oct 28, 2020

Conversation

spell00
Copy link
Contributor

@spell00 spell00 commented Sep 1, 2020

Brief summary of changes

Transition to v0.0.4-dev to include new extensions of the API that are not in the current specs.
The changes include:

  • The specs of v0.0.3 is copied into v0.0.4-dev
  • The integration tests are changed to test v0.0.4-dev
  • All current endpoints allow v0.0.4-dev of the API

Link(s) to related issue(s)

@spell00 spell00 force-pushed the 2020-09-01-NewApiVersion_0.0.4 branch from 801d93b to 72b1e96 Compare September 1, 2020 19:16
@spell00 spell00 changed the title 2020 09 01 new api version 0.0.4 Transition to API version 0.0.4 Sep 1, 2020
@spell00 spell00 added API PR or issue introducing/requiring modifications to the API code Blocking PR should be prioritized because it is blocking the progress of another task labels Sep 1, 2020
@spell00 spell00 added the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Sep 2, 2020
@spell00
Copy link
Contributor Author

spell00 commented Sep 2, 2020

The version new name needs to be discussed.

@spell00 spell00 removed the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Sep 14, 2020
@spell00
Copy link
Contributor Author

spell00 commented Sep 16, 2020

@xlecours Could you please have a look at this?

Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add something in CHANGELOG.md to mention that there is a new version of the api specification in the modules section. We will add items under it for each new or changed endpoints.

@spell00 spell00 force-pushed the 2020-09-01-NewApiVersion_0.0.4 branch from 6b6d928 to e4e5061 Compare September 18, 2020 18:04
@xlecours
Copy link
Contributor

@driusan This PR set the table for additional endpoints and modifications of existing endpoints. I will evaluate if the following PR can be include in v0.0.4
#6905
#6899
#6780
#6775
#6161
#5354

Meanwhile, I think this PR is the first step. Would you prefer to have a feature branch instead?

Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there's still other PRs to go into 0.0.4, please add the -dev suffix to the version number until it's ready. Otherwise we'll have multiple incompatible versions of "0.0.4".

Other than that it looks good to me.

@xlecours
Copy link
Contributor

xlecours commented Sep 23, 2020

Since there's still other PRs to go into 0.0.4, please add the -dev suffix to the version number until it's ready. Otherwise we'll have multiple incompatible versions of "0.0.4".

Other than that it looks good to me.

@driusan it is not released.It is implicitly -dev. Otherwise, we would need to change all the 0.0.4 to 0.0.4-dev then back to 0.0.4 before the release.
And this is still v0

@spell00
Copy link
Contributor Author

spell00 commented Sep 29, 2020

@xlecours @driusan So what should it be? Is -dev because there is not a new version of the API at each release? How is it decided when it is time to remove the -dev suffix and start a new version? It's an easy and fast update to do, I wonder if updating the version systematically on each release wouldn't be faster and more straightforward, as it would avoid having a discussion on every release about removing or adding -dev?

Also, I thought it was implicitly -dev too, because it is v0. Wouldn't it make sense if all official new version of the API would update the first number (e.g. v0.0.4 -> v1.0.0) and all updates of the last number would be implicitely a dev version (e.g. v1.0.0 -> 1.0.1 -> 1.0.2)?

@spell00 spell00 requested a review from driusan October 6, 2020 15:51
@christinerogers
Copy link
Contributor

christinerogers commented Oct 6, 2020

@driusan i think the naming is now resolved per today's loris meeting.
Could you please re-review?

This PR is holding up several other PRs that need to be merged in the next 2 weeks, plus key for the release.
cc @xlecours

@christinerogers christinerogers added the Critical to release PR or issue is key for the release to which it has been assigned label Oct 6, 2020
@@ -1,5 +1,4 @@
# Loris API - v0.0.2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like I modified a lot of things in the doc of v0.0.2, but it is actually modified from v0.0.3. I removed LorisRESTAPI.md and added LorisRESTAPI_v0.0.4-dev.md.

I did not modifiy the original LorisRESTAPI_v0.0.3.md so Github must consider I replaced v0.0.2 by v0.0.4-dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I solved this problem by renaming LorisRESTAPI_v0.0.3.md to LorisRESTAPI.md.

@@ -75,8 +75,10 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator
*/
protected function supportedVersions() : array
{
// Removed 0.0.2 since session requires a project.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is no longer relevant, since 0.0.2 is no longer supported anywhere

@spell00
Copy link
Contributor Author

spell00 commented Oct 20, 2020

@driusan @xlecours as per our discussion this morning, I have removed support for v0.0.2 in all endpoints. I also removed the doc for v0.0.2.

I am not sure if @xlecours agreed with the use of the -dev suffixe, but I think we should proceed with this PR as it is (with -dev). If we were to decide to change the version to remove -dev, it is very fast to change it.

From the directory modules/api/php/endpoints/, I would just execute the command line:

grep -rli 'v0.0.4-dev' * | xargs -i@ sed -i 's/v0.0.4-dev/v0.0.4/g' @

cc @christinerogers

@christinerogers
Copy link
Contributor

@xlecours @driusan could we get some joint attention on this PR? It's blocking the rest of Simon's work from moving forward, and this needs to be resolved in time.

@xlecours
Copy link
Contributor

ok @driusan let's move forward.

@driusan driusan merged commit 7885224 into aces:main Oct 28, 2020
spell00 added a commit to spell00/Loris that referenced this pull request Nov 4, 2020
Initiate development on version 0.0.4-dev of the API and remove support for 0.0.2

    Resolves aces#6941
@ridz1208 ridz1208 added this to the 24.0.0 milestone Nov 27, 2020
driusan added a commit to driusan/Loris that referenced this pull request Nov 27, 2020
driusan added a commit that referenced this pull request Dec 3, 2020
…e v0.0.3 (#7150)

v0.0.2 support was removed in #6944.

The CHANGELOG was also referencing the wrong pull request.
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
Initiate development on version 0.0.4-dev of the API and remove support for 0.0.2

    Resolves aces#6941
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
…e v0.0.3 (aces#7150)

v0.0.2 support was removed in aces#6944.

The CHANGELOG was also referencing the wrong pull request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API PR or issue introducing/requiring modifications to the API code Blocking PR should be prioritized because it is blocking the progress of another task Critical to release PR or issue is key for the release to which it has been assigned
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API] New version of the API (v0.0.4)
5 participants